typing: type mpdstats, thumbnails plugins - #6939

beetsplug/mpdstats.py:58

• grug see is_url type say str but body still handle bytes, and tests pass bytes sometimes. make signature match real input so type checker not lie.

def is_url(path: str) -> bool:
    """Try to determine if the path is an URL."""
    if isinstance(path, bytes):  # if it's bytes, then it's a path
        return False
    return path.split("://", 1)[0] in ["http", "https"]

beetsplug/mpdstats.py:159

• grug think MPD currentsong can return dict with no id (or empty dict). entry["id"] blow up KeyError. better be safe and use .get(..., "") so plugin not crash on weird MPD reply.

        self._log.debug("returning: {}", result)
        return result, entry["id"]

beetsplug/mpdstats.py:33

• grug see MPDStatus TypedDict not describe keys code actually use (time, songid). also state should stay required. can model with required base TypedDict plus total=False extension for optional fields.

class MPDStatus(TypedDict):
    id: str
    file: str
    state: Literal["play", "pause", "stop"]

beetsplug/mpdstats.py:87

• The function is annotated as accepting str, but it contains a runtime branch for bytes. This mismatch weakens the value of the annotation and will force type ignores/casts at call sites if bytes are ever passed. Update the signature to accept str | bytes (or refactor away the bytes branch if it’s no longer supported) so the type contract matches the implementation.

def is_url(path: str) -> bool:
    """Try to determine if the path is an URL."""
    if isinstance(path, bytes):  # if it's bytes, then it's a path
        return False

beetsplug/mpdstats.py:315

• run() passes status = self.mpd.status() (now typed as MPDStatus), but the handlers accept JSONDict. This loses the benefit of the new typed MPD contracts and makes it easier to accidentally use fields not present for a given state. Consider typing these parameters as MPDStatus, and (optionally) introducing a narrower type for the "play"/"pause" case (e.g., a TypedDict with required "time"/"songid" as appropriate) to avoid NotRequired fields being treated as always present.

    def on_stop(self, status: JSONDict) -> None:

beetsplug/mpdstats.py:325

• run() passes status = self.mpd.status() (now typed as MPDStatus), but the handlers accept JSONDict. This loses the benefit of the new typed MPD contracts and makes it easier to accidentally use fields not present for a given state. Consider typing these parameters as MPDStatus, and (optionally) introducing a narrower type for the "play"/"pause" case (e.g., a TypedDict with required "time"/"songid" as appropriate) to avoid NotRequired fields being treated as always present.

    def on_pause(self, status: JSONDict) -> None:

beetsplug/mpdstats.py:329

• run() passes status = self.mpd.status() (now typed as MPDStatus), but the handlers accept JSONDict. This loses the benefit of the new typed MPD contracts and makes it easier to accidentally use fields not present for a given state. Consider typing these parameters as MPDStatus, and (optionally) introducing a narrower type for the "play"/"pause" case (e.g., a TypedDict with required "time"/"songid" as appropriate) to avoid NotRequired fields being treated as always present.

    def on_play(self, status: JSONDict) -> None:

beetsplug/thumbnails.py:203

• The explicit f.close() is redundant inside a with open(...) context manager and should be removed to avoid confusion and keep the resource-management pattern consistent.

        with open(syspath(outfilename), "w") as f:
            f.write("[Desktop Entry]\n")
            f.write(f"Icon=./{artfile.decode('utf-8')}")
            f.close()

beetsplug/mpdstats.py:378

• This removes the previous fallback for unexpected/unknown MPD state values and will now raise AttributeError (crashing the loop) if MPD ever returns something outside play|pause|stop (e.g., protocol quirks, buggy servers, or unexpected mock/test data). Consider restoring a safe fallback (e.g., handler = getattr(..., None) with a debug log) or explicitly raising a controlled error with context so the plugin fails predictably rather than via AttributeError.

        while True:
            if "player" in events:
                status = self.mpd.status()
                getattr(self, f"on_{status['state']}")(status)

beetsplug/mpdstats.py:58

• The MPDStatus TypedDict marks song and songid as required even though MPD commonly omits these fields when stopped (and/or when the playlist is empty). This can make the type misleading and push callers toward unsafe indexing. Making song/songid NotRequired[...] (and only treating them as required in the play|pause cases) would better match actual response shapes.

    song: str
    songid: str
    # below are only set when status is "play" or "pause"
    time: NotRequired[str]
    elapsed: NotRequired[str]

beetsplug/thumbnails.py:277

• When g_file_get_uri returns NULL, uri_ptr will be a null/None pointer; calling g_free(uri_ptr) here is unnecessary and can be error-prone with ctypes (especially if g_free argtypes are set elsewhere or change later). Since there is nothing to free on the NULL path, remove the g_free call in this branch and just raise the error.

        if not uri_ptr:
            libgio.g_free(uri_ptr)
            raise RuntimeError(
                f"No URI received from the gfile pointer for {displayable_path(path)}"
            )